Skip to content

Refactor postgresql migration #219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 2, 2017
Merged

Conversation

ssh24
Copy link
Contributor

@ssh24 ssh24 commented Feb 23, 2017

Description

Refactor discovery test cases

Related issues

connect to loopbackio/loopback-connector#15

@ssh24 ssh24 added the review label Feb 23, 2017
@ssh24 ssh24 changed the title Refactor postgresql migration [DO NOT MERGE - WIP] Refactor postgresql migration Feb 24, 2017
@ssh24 ssh24 force-pushed the refactor-postgresql-migration branch 6 times, most recently from 320aa34 to 6f79234 Compare February 28, 2017 00:41
lib/migration.js Outdated
}
if (!err) {
data.forEach(function(field) {
field.type = mapPostgreSQLDatatypes(field.type, field.length);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mapPostgreSQLDatatypes is missing after refactor

@ssh24 ssh24 force-pushed the refactor-postgresql-migration branch from 7ad2413 to 760e761 Compare March 1, 2017 23:23
* @param {String} property The property name
* @returns {String} The column type
*/
PostgreSQL.prototype.columnDataType = function(model, property) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, why changing this function from columnDataType to buildColumnType?
we need both of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved columnDataType to base connector loopbackio/loopback-connector@752359f
We still have buildColumnType in postgresql

@ssh24 ssh24 changed the title [DO NOT MERGE - WIP] Refactor postgresql migration Refactor postgresql migration Mar 2, 2017
@ssh24
Copy link
Contributor Author

ssh24 commented Mar 2, 2017

@slnode test please

@ssh24 ssh24 force-pushed the refactor-postgresql-migration branch from 760e761 to 15c20ae Compare March 2, 2017 18:34
@ssh24 ssh24 requested a review from raymondfeng March 2, 2017 18:43
Copy link
Contributor

@raymondfeng raymondfeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Extract getTableStatus & autoupdate

Extract isActual function

Extract getDropColumns into base connector

Extract addPropertyToActual to base connector

Extract searchForPropertyInActual to base

Extract propertyHasNotBeenDeleted to base

Extract applySqlChanges to base

Extract columnDataType to base connector

Use mapPostgreSQLDatatypes in showFields
@ssh24 ssh24 force-pushed the refactor-postgresql-migration branch from 15c20ae to d6b1504 Compare March 2, 2017 18:52
@ssh24 ssh24 merged commit 62dc458 into master Mar 2, 2017
@ssh24 ssh24 deleted the refactor-postgresql-migration branch March 2, 2017 19:06
@ssh24 ssh24 removed the review label Mar 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants